-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect Authorization plugin and include it in bake output #993
Conversation
Add the Authorization component if no explicit list of components was requested.
I had to fix line endings in several files as our templates were missing terminating newlines and that's no way to live.
Remove editorconfig that was doing the wrong thing and removing trailing newlines.
.editorconfig
Outdated
@@ -16,6 +16,3 @@ indent_size = 2 | |||
|
|||
[phars.xml] | |||
indent_size = 2 | |||
|
|||
[*.twig] | |||
insert_final_newline = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was some file that needed this, but I can't remember which one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah whitespace changes in templates can affect the display, so I wouldn't auto add/remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markstory Can you revert this please.
@@ -26,6 +26,9 @@ | |||
->contain({{ Bake.exportArray(belongsTo)|raw }}); | |||
{% else %} | |||
$query = $this->{{ currentModelName }}->find(); | |||
{% endif %} | |||
{% if Bake.hasPlugin('Authorization') %} | |||
$query = $this->Authorization->applyScope($query); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to bake stub code for these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is helpful. I usually use this to build in permissions like 'view all the projects I have membership in'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, should we bake/generate the authorization code stubs to match this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, should we bake/generate the authorization code stubs to match this?
That can be done already with bake policy --type table Articles
. If we wanted to get fancy we could make the bake code generation respect whether or not you have a table/entity policy generated.
@@ -22,6 +22,9 @@ | |||
public function add() | |||
{ | |||
${{ singularName }} = $this->{{ currentModelName }}->newEmptyEntity(); | |||
{% if Bake.hasPlugin('Authorization') %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious about the use case for authorizing using an empty entity. Personally I have only authorized the "add" action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applications could have roles or rules on who can create what. I agree that most of the time add() will have a policy of return true
.
Anything else I should take care of with this? I'd like to get this merged so I can move forwards with improving the authentication integration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except the change to .editorconfig
.
.editorconfig
Outdated
@@ -16,6 +16,3 @@ indent_size = 2 | |||
|
|||
[phars.xml] | |||
indent_size = 2 | |||
|
|||
[*.twig] | |||
insert_final_newline = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markstory Can you revert this please.
If the host application has Authorization plugin installed, we can autodetect that and include component checks in generated code from bake.